fix(litellm): preserve reasoning_content for known reasoning model families#899
fix(litellm): preserve reasoning_content for known reasoning model families#899daewoongoh wants to merge 2 commits into
Conversation
…milies LiteLLM's /v1/model/info never reports reasoning capability flags, so infer preserveReasoning from the model alias/routed-model name and use convertToR1Format to keep reasoning_content intact across tool-call turns. Signed-off-by: daewoongoh <dw.oh@samsung.com>
…on branching Add unit coverage for LITELLM_PRESERVE_REASONING_PATTERN matching per model family, getLiteLLMModels setting preserveReasoning on matched models, and LiteLLMHandler.createMessage branching between convertToR1Format and convertToOpenAiMessages based on info.preserveReasoning. Signed-off-by: daewoongoh <dw.oh@samsung.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughLiteLLM model aliases are now matched against known reasoning families. Matching models receive ChangesLiteLLM reasoning preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant getLiteLLMModels
participant LiteLLMHandler
participant convertToR1Format
participant LiteLLM API
getLiteLLMModels->>getLiteLLMModels: Match model name and routed alias
getLiteLLMModels-->>LiteLLMHandler: Return preserveReasoning metadata
LiteLLMHandler->>convertToR1Format: Convert messages and merge tool results
convertToR1Format-->>LiteLLMHandler: Return reasoning-preserving messages
LiteLLMHandler->>LiteLLM API: Send converted conversation
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
edelauna
left a comment
There was a problem hiding this comment.
Thanks for this contribution, had a small comment on test and the regex in use.
| String.raw`kimi-k2\.7-code`, // fireworks.ts (dotted alias variant) | ||
| String.raw`minimax[.-]?m[23](\.\d+)?(-highspeed|-stable)?\b`, // bedrock.ts, minimax.ts, opencode-go.ts | ||
| String.raw`glm-4\.7(?!-flash)\b`, // zai.ts (excludes glm-4.7-flash(x)) | ||
| String.raw`glm-5(\.[12])?(-turbo)?(?!-flash)\b`, // zai.ts, opencode-go.ts |
There was a problem hiding this comment.
Could this still match glm-5 as a prefix for names like glm-5.1-flash or glm-5.3? If those should stay excluded, it may be worth making this fragment consume the accepted suffix fully and adding negative cases for those variants.
| data: { | ||
| data: [ | ||
| { | ||
| model_name: "deepseek-reasoner-alias", |
There was a problem hiding this comment.
Could this alias be something neutral that does not match the preserve-reasoning pattern? As written, this still passes if the implementation only checks model_name, so it does not quite prove that litellm_params.model is used.
Related GitHub Issue
Closes: #891
Description
LiteLLM's
/v1/model/inforesponse never exposes reasoning-related capability flags, sopreserveReasoningwas always resolving tofalsefor reasoning/thinking models routed through a LiteLLM proxy (DeepSeek-Reasoner, GLM-4.7/5, Kimi-K2-Thinking, MiniMax-M2/M3, Qwen3-plus/max, etc.). As a result,LiteLLMHandler.createMessagealways usedconvertToOpenAiMessages, soreasoning_contentfrom a prior turn was never echoed back to the API after a tool-call turn. This differs from accessing the same underlying models directly through their native provider config (e.g.deepseek.tsalready setspreserveReasoning: true), and some providers reject the request with a 400 error when required reasoning history is missing.How:
packages/types/src/providers/lite-llm.ts: Collected the alias/routed-model-name patterns for every model family that requirespreserveReasoning: truein its native provider config (deepseek.ts,mimo.ts,moonshot.ts,bedrock.ts,fireworks.ts,zai.ts,minimax.ts,opencode-go.ts), and exported them asLITELLM_PRESERVE_REASONING_PATTERN.src/api/providers/fetchers/litellm.ts:getLiteLLMModelsnow concatenatesmodel_nameandlitellm_params.modeland tests them against that pattern while parsing each model, settingModelInfo.preserveReasoning: trueon a match.src/api/providers/lite-llm.ts:createMessagenow branches oninfo.preserveReasoning—trueroutes throughconvertToR1Format(withmergeToolResultText: true), otherwise it keeps usingconvertToOpenAiMessages.convertToR1Formatfolds any text following a tool_result into the preceding tool message instead of inserting a separate user message, which prevents reasoning-mode providers from droppingreasoning_contentwhen a user message appears mid-turn.Trade-off: the matching is a string-pattern heuristic. Since LiteLLM never surfaces the routed model's actual capabilities, this can't be fully accurate — unrecognized aliases or renamed deployments simply won't match (documented in the
lite-llm.tscomment).Test Procedure
pnpm vitest run(unit tests):packages/types/src/__tests__/lite-llm.test.ts: verifiesLITELLM_PRESERVE_REASONING_PATTERNmatches/doesn't match each model family (DeepSeek, MiMo, Kimi-K2, MiniMax-M2/M3, GLM-4.7/5, Qwen3).src/api/providers/fetchers/__tests__/litellm.spec.ts: verifiesgetLiteLLMModelssetspreserveReasoning: truewhen either the alias (model_name) or the routed model name (litellm_params.model) matches, and omits the field otherwise.src/api/providers/__tests__/lite-llm.spec.ts: verifiesconvertToR1Formatis used andreasoning_contentsurvives on the outgoing request whenpreserveReasoning: true; and that the non-preserveReasoningbranch sends noreasoning_content(reflecting thatTask.buildCleanConversationHistoryalready strips the reasoning block upstream in that case).litellm_params.modelrouted to a reasoning model (e.g.deepseek/deepseek-reasoner), run a multi-turn task involving a tool call — previously the follow-up request droppedreasoning_content; it's now preserved.Pre-Submission Checklist
Documentation Updates
Additional Notes
If a reasoning model family or renamed alias isn't caught by this heuristic, additional patterns can be appended to
LITELLM_PRESERVE_REASONING_FRAGMENTS.Get in Touch
hehegwk_23849
Summary by CodeRabbit
New Features
Bug Fixes
Tests